[Impeller] Add support for specialization constants redux. #47678
Conversation
…/engine into add_specialization_support
| ToMTLPixelFormat(desc.GetStencilPixelFormat()); | ||
|
|
||
| return descriptor; | ||
| const auto& constants = desc.GetSpecializationConstants(); |
There was a problem hiding this comment.
This is still a bit iffy, but we improve performance by not blocking on the specialization, it all goes into the same pipeline future which is only blocking once we actually trry to use it.
Since the compilation is out of process anyway, blocking on it is pointless and only serves to park the thread.
There was a problem hiding this comment.
Feels like we're writing javascript here with this chain of nested completion callbacks. ;) Looks safe to me, though.
| promise->set_value(new_pipeline); | ||
| }; | ||
| auto mtl_descriptor = GetMTLRenderPipelineDescriptor(descriptor); | ||
| #if FML_OS_IOS |
There was a problem hiding this comment.
Todo: add back
There was a problem hiding this comment.
Actually, i see this issue is closed. Is this intended to be removed now?
jonahwilliams
changed the title
bdero
left a comment
bdero
left a comment
There was a problem hiding this comment.
LGTM modulo latch question. Crasher fix & new changes make sense.
| ToMTLPixelFormat(desc.GetStencilPixelFormat()); | ||
|
|
||
| return descriptor; | ||
| const auto& constants = desc.GetSpecializationConstants(); |
There was a problem hiding this comment.
Feels like we're writing javascript here with this chain of nested completion callbacks. ;) Looks safe to me, though.
| return descriptor; | ||
| // This latch is used to ensure that GetMTLFunctionSpecialized does not finish | ||
| // before the descriptor is completely set up. | ||
| auto latch = std::make_shared<fml::CountDownLatch>(1u); |
There was a problem hiding this comment.
Nothing appears to be waiting on this latch, and I'm not sure why we'd need one. It looks like the descriptor is already done being set up by the time we call GetMTLFunctionSpecialized.
There was a problem hiding this comment.
Removed. This was left over from an earlier iteration.
jonahwilliams
added
the
autosubmit
jonahwilliams
added
the
revert
auto-submit
bot
removed
the
revert
2 participants
Reland of #47432
Also includes:
Fixes the performance on iOS by removing blocking on compilation of shaders. From local testing this has identical before/after numbers. Additional, ensures that we don't unecessarily specialize vertex shaders and notes this restriction in the documentation.
Adds support for Specialization constants to Impeller for our usage in the engine. A motivating example has been added in the impeller markdown docs.
Fixes flutter/flutter#136210
Fixes flutter/flutter#119357